Skip to content

fix: accept native dense KV skip evidence in FPM collection - #529

Open
simone-chen wants to merge 5 commits into
mainfrom
simonec/fix-dense-fpm-skip-compat
Open

simone-chen wants to merge 5 commits into
mainfrom
simonec/fix-dense-fpm-skip-compat

Conversation

@simone-chen

Copy link
Copy Markdown
Contributor

Why and what changed

  • Dynamo #15110 reports deliberately skipped dense KV warm-up as per-point skip:dense_model_content_insensitive, without the older kvwarm_fake_fallback stamp. AISimulate main currently converts these otherwise qualified measurements to fake_fallback, leaving dense decode unusable.
  • Consume the validated per-point regime during formal aggregation. Accept either producer format only when the existing dense full-attention, model/runtime, topology and warm-up checks pass. Missing or contradictory evidence remains ineligible.
  • Keep measured latencies, raw artifacts and historical publication policy intact. Document both formats and cover their readiness and publication behavior. Worker-probe sidecar recognition already exists on main; this PR adds combined coverage.

Review map

  • Risk level: low; narrowly scoped evidence classification with unchanged eligibility checks.
  • Start with collector/fpm_forward/database.py::aggregate_cell and tests/unit/collector/test_fpm_dense_synthetic_kv.py under python/aisimulate/.
  • Public or serialized contract changed: no new schema or fields; consume an existing Dynamo per-point regime.
  • Compatibility: retain legacy stamping and explicit historical publication policy. No Dynamo change or AISimulate release cherry-pick is included.

Evidence

  • Tests: 394 passed across test_fpm_dense_synthetic_kv.py, test_fpm_worker_probe_artifact.py, test_fpm_forward.py and test_fpm_measurement_evidence.py, using python -m pytest -c python/aisimulate/pytest.ini -p no:timeout with this worktree's Python source on PYTHONPATH.
  • Before/after: three new-format full-attention cases failed on the original implementation and pass with the fix; the three corresponding legacy cases already passed.
  • Negative cases: missing/mismatched engine identity, sliding/chunked/non-causal/recurrent caches, disabled or conflicting warm-up, absent point evidence, and missing warm-up metadata remain ineligible. The public readiness test accepts 12/12 qualified points in both formats and 0/12 without qualifying cache evidence.
  • Ruff lint, Ruff format check and git diff --check passed.
  • Fast CI / Full CI: pending; Full CI will be requested for the pushed head.
  • CodeRabbit: pending. Codex: local diff reviewed; no independent agent review.

Modeling or data provenance

  • No measured data or latency arithmetic changed. Tests use existing synthetic fixtures and preserve their raw bytes and 10/20 ms values. No new GPU collection or serving-accuracy validation was performed.

Tracking

Signed-off-by: Simone Chen <simonec@nvidia.com>
@simone-chen
simone-chen requested review from a team as code owners October 20, 2026 20:42
@simone-chen

Copy link
Copy Markdown
Contributor Author

/ok to test 8d62746

@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Summary

Risk: Medium. The production change is small (7 lines in database.py). It changes how formal aggregation classifies a native dense skip point. Human review should focus on:

  1. New native-regime branch in _kv_seed_regime. A point with native skip:dense_model_content_insensitive returns early. It does not run the sample_reasons checks that the legacy path uses. Eligibility for this branch depends only on the dense_synthetic flag. Confirm that _dense_synthetic_kv_eligible covers every check the legacy path enforced.
  2. Eligibility gate. _dense_synthetic_kv_eligible is unchanged in this diff. It now controls both the legacy kvwarm_fake_fallback path and the native path. Verify its full-attention, model/runtime, topology, and warm-up checks against the docs.
  3. Test coverage for negative cases. The test diff (+65/-17 in test_fpm_dense_synthetic_kv.py) adds native-skip cases. Confirm that malformed or missing evidence, failed or disabled warm-up, and unstamped points still produce fake_fallback.

Changed behavior:

  • A native skip:dense_model_content_insensitive point is kept only when dense synthetic KV is enabled. Otherwise it is classified as fake_fallback.
  • Prefill points with no execution identity or zero KV-read tokens still return n/a first.
  • The documentation now allows the same skip in two formats: the legacy kvwarm_fake_fallback stamp, or the native per-point regime with no fallback stamp. Both require the same engine evidence. A configuration-level skip reason alone does not qualify a point. Worker-probe sidecars do not replace the engine snapshot.
  • No schema, field, or public declaration changes. The two README edits only remove trailing whitespace.

Evidence supplied:

  • The merge-base diff confirms 5 files, +74/-21, and the database.py and documentation changes listed above.
  • The author reports 394 passing tests across four test files. The author also reports that the readiness test accepts 12/12 qualified points in both formats and 0/12 without qualifying cache evidence. I did not run these tests. Treat them as author-reported.
  • The author reports that Ruff lint, Ruff format check, and git diff --check passed.

Evidence missing:

  • Fast CI and Full CI were pending in the source material. No completed check result on the current head was available to me.
  • No GPU collection or serving-accuracy validation was performed. The docs state that the change does not establish real KV content or serving accuracy.
  • I did not inspect the test file diff line by line.

Technical quality: The change is narrow and matches its documented contract. Its main risk is the early return that skips per-point sample_reasons checks.

Merge readiness: Not established. CI is pending, and the test results are author-reported. Do not treat any bot review as approval. Merge after CI completes and the negative-case tests in item 3 are confirmed.

Collector aggregation now receives each measurement’s native KV seed regime and retains eligible dense skips. Tests cover native and legacy inputs across eligibility, publication, and readiness. The readiness documentation describes accepted per-point representations and their engine evidence requirements.

Changes: Dense synthetic KV classification

Layer / File(s) Summary
Native regime classification
python/aisim/collector/fpm_forward/database.py, docs/perf-model/fpm-self-service/implementation.md
Aggregation passes the native regime to _kv_seed_regime. Eligible native dense skips are retained; the documentation describes accepted representations and required engine evidence.
Eligibility and publication tests
python/aisim/tests/unit/collector/test_fpm_dense_synthetic_kv.py
Tests cover native and legacy samples across eligibility and evidence checks, warmup, publication, and readiness classifications.

Collector README whitespace edits

Layer / File(s) Summary
Remove trailing spaces
python/aisim/collector/network/slurm/README.md, python/aisim/collector/sglang/README.md
The NCCL collection heading and DEEPSEEK_MODEL_PATH entry no longer have trailing spaces.

Estimated code review effort: 2 (Simple) | ~12 minutes

Possibly related PRs

  • ai-dynamo/aisim#778: Updates decoder-config selection in the dense synthetic KV eligibility path, which is also involved in the eligibility checks covered here.
Merge Risk: Minimal · up to 60824

This change lets the collector accept the newer dense KV skip evidence while keeping the existing eligibility checks. No concrete merge-blocking risk was found. Wait for CI to finish before merging.

Pre-merge checks: Passed 8

Check name Status Explanation
Title check Passed The title names the behavior change: the collector now accepts native dense KV skip evidence. It matches the summarized changes to aggregate_cell and the dense synthetic-KV tests. It does not use a va…
Description check Passed The description follows the template sections: Why and what changed, Review map with risk level and contract notes, Evidence with test commands and results, Modeling or data provenance, and Tracking. …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Cross-Layer Contract Passed PR #529 (ai-dynamo/aisim, author simone-chen) changes one input contract: a per-point native kv_seed_regime: skip:dense_model_content_insensitive is now consumed by aggregate_cell in `python/aisim…
Modeling And Data Evidence Passed The check applies. The PR changes selection logic: aggregate_cell in python/aisim/collector/fpm_forward/database.py now reads each measurement's native kv_seed_regime and promotes `skip:dense_mo…
Compatibility Boundaries Passed The PR does not change any compatibility surface that the check covers. - Changed files (name-status): one doc (docs/perf-model/fpm-self-service/implementation.md, +2 lines), `python/aisim/collector…
Review Evidence Passed The PR description meets the "Review evidence" check. - Commands and results: It names the exact local command (python -m pytest -c python/aisimulate/pytest.ini -p no:timeout), the test files co…
Full details: Title check

Explanation

The title names the behavior change: the collector now accepts native dense KV skip evidence. It matches the summarized changes to aggregate_cell and the dense synthetic-KV tests. It does not use a vague term, though it uses a prefix that the repository does not require.

Full details: Description check

Explanation

The description follows the template sections: Why and what changed, Review map with risk level and contract notes, Evidence with test commands and results, Modeling or data provenance, and Tracking. It states the risk level as low, names the start files, and says no public or serialized contract changed. Evidence for Fast CI, Full CI, and CodeRabbit is still pending. The Codex reviewed commit field is blank even though the risk is low, so the template's required-for-medium/high rule does not apply. The Feature Support Matrix field is not answered. The Review map lists the Dynamo PR and release companion but not a clear compatibility rollback plan.

Resolution

Optional: state the Feature Support Matrix status (unaffected or updated), fill in the CodeRabbit reviewed commit once known, and add a short rollback note. Fast CI and Full CI results are pending and should be recorded when they finish.

Full details: Cross-Layer Contract

Explanation

PR #529 (ai-dynamo/aisim, author simone-chen) changes one input contract: a per-point native kv_seed_regime: skip:dense_model_content_insensitive is now consumed by aggregate_cell in python/aisim/collector/fpm_forward/database.py (lines 267-272, 308). The changed input and its consumers were traced: - Producer: Dynamo HEAD components/src/dynamo/vllm/instrumented_scheduler.py _kvwarm_seed_regime (around lines 5793-5830) returns skip:{skip_reason} for warm-ineligible decode points. The dense reason is set at line 5916. Per-point kv_seed_regime is written at lines 7635, 8053 and 8062. The format the PR consumes matches the producer. - Validation: native_artifact.py _validate_seed_regime (lines 49-76) accepts the new value with no stamp and rejects stamp contradictions. It is unchanged by the PR. validate_native_collection runs inside aggregate_cell, so the new path is exercised. - Aggregation: the new branch returns the skip only when the config-level dense gate (dense_synthetic) passes. Otherwise it returns fake_fallback. Tests cover both branches. Positive tests show promotion. Negative tests cover unknown or non-full-attention caches, failed or disabled warm-up, contradictory or missing engine identity, and missing kvwarm metadata. Each runs with native_skip both on and off. - Publication and readiness: kv_seed_classification metadata, validate_formal_database_commit and collection_readiness counts are covered by parametrized tests (test_dense_publication_*, test_public_readiness_*). Legacy dense_synthetic_kv=False is covered. - Documentation: docs/perf-model/fpm-self-service/implementation.md (lines 1252) documents both formats and the engine-evidence requirement. python/aisim/collector/README.md (lines 249-253) still lists the accepted regimes. Its text ("an approved skip:<reason>", "non-benign warm-up skips remain fake_fallback") remains accurate after the change. It does not name the dense policy. Residual gaps (not failures under this check): - The test fixtures are hand-built via _use_native_dense_skip. No captured Dynamo artifact pins the format. The Dynamo source supports the format, but the aisim tests do not run against real producer output. - Dynamo HEAD also emits unstamped, skip:peer_ineligible, and *_attention_kv_random_kda regimes. The PR does not change how aisim handles these. The PR wording "newer main output contract" is broader than the dense regime it actually covers. - Native-skip plus conflicting stamp is rejected by the pre-existing validator, but no test in the PR asserts that rejection. - collector/README.md still states metadata schema v6, while database.py writes schema_version 7. This predates the PR. Test results (394 passed) are author-reported. I did not execute them.

Resolution

Non-blocking follow-ups: 1. Add one fixture or contract test that uses captured Dynamo output (or a pinned Dynamo commit reference) for the per-point skip:dense_model_content_insensitive format. 2. Add tests for native-skip plus kvwarm_fake_fallback stamp contradiction and for DP-rank disagreement between skip and stamped regimes. 3. Narrow the PR wording to the dense per-point regime, or document how the remaining Dynamo regimes (unstamped, skip:peer_ineligible, *_random_kda) are handled. 4. Optionally mention the dense_full_attention_v1 policy in python/aisim/collector/README.md and correct its schema version.

Full details: Modeling And Data Evidence

Explanation

The check applies. The PR changes selection logic: aggregate_cell in python/aisim/collector/fpm_forward/database.py now reads each measurement's native kv_seed_regime and promotes skip:dense_model_content_insensitive only when dense synthetic KV is enabled. Otherwise it is classified as fake_fallback. The diff is small (+5/-2) and matches the PR description. No measured data, latency values or published data changed. The PR gives reproducible before/after evidence for this logic: - Command: python -m pytest -c python/aisimulate/pytest.ini over four named test files, with 394 passing. - Before/after: three new-format full-attention cases fail on the original code and pass with the fix. The three legacy cases pass both before and after. - Outcome counts: the public readiness test accepts 12/12 qualified points in both formats and 0/12 without qualifying cache evidence. - Negative cases cover missing or mismatched engine identity, unsupported cache types, disabled or conflicting warm-up, and missing point evidence. The fixtures are existing synthetic ones and are in the repository, so the tests can be rerun. The PR does not claim a serving-accuracy or GPU validation, and the check does not require one for a classification change. The before/after evidence is test-based and not a golden data diff. The before-state counts for the native format are implied rather than stated, which is a minor gap. The evidence is still explained and reproducible, so the check passes. I could not find the dense_model_content_insensitive string in the linked aiconfigurator reference repository. That search is not evidence about this check, because the change concerns the Dynamo and aisim producer contract.

Full details: Compatibility Boundaries

Explanation

The PR does not change any compatibility surface that the check covers. - Changed files (name-status): one doc (docs/perf-model/fpm-self-service/implementation.md, +2 lines), python/aisim/collector/fpm_forward/database.py (_kv_seed_regime now takes the native kv_seed_regime), two README whitespace fixes, and test_fpm_dense_synthetic_kv.py. - No changes to Cargo.toml, crates/*/Cargo.toml, python/aisim/pyproject.toml, or any .rs, .toml, .json, .yaml file. Versions remain 0.15.0 for the Python package and the core crate. No binding, schema, or public API declaration changes. - The new code path compares against the literal skip:dense_model_content_insensitive. That literal already exists on the base branch in database.py (base lines 292, 418, 840) and in the existing tests. The PR adds no new Dynamo import and no new Dynamo runtime dependency. The author states that no new field or schema is introduced. - Standalone status: Dynamo imports in runtime/ (for example dynamo.vllm.instrumented_scheduler) are present on the base branch. The PR does not add or modify them, so they are not caused by this PR. This check does not require removing them. - The documentation addition describes both producer formats and their shared engine-evidence requirement. It does not change any documented default. - The one-wheel plus one-crate artifact contract is unchanged, because no packaging or build files changed. Residual note (not a failure): the PR consumes a Dynamo-produced regime string. Behavior depends on the external value remaining stable. This is an existing coupling, and this check does not cover it.

Full details: Review Evidence

Explanation

The PR description meets the "Review evidence" check. - Commands and results: It names the exact local command (python -m pytest -c python/aisimulate/pytest.ini -p no:timeout), the test files covered, and the result (394 passed). It also lists Ruff lint, Ruff format check, and git diff --check as passed. - Negative and boundary cases: It lists ineligible cases: missing or mismatched engine identity, sliding, chunked, non-causal, and recurrent caches, disabled or conflicting warm-up, absent point evidence, and missing warm-up metadata. It also states the readiness result of 12/12 qualified points accepted and 0/12 without qualifying cache evidence. - Before/after evidence: It reports that three new-format cases failed on the original code and pass with the fix. - Evidence types: It separates local test runs from production evidence. It states that tests use existing synthetic fixtures and that no new GPU collection or serving-accuracy validation was performed. - CI citation: Fast CI and Full CI are described as pending. The description does not cite any inactive nested workflow as hosted CI evidence, so the failure condition does not apply. Gaps: the description does not explicitly label parity evidence. This does not trigger a FAIL condition, and the description does distinguish the local, synthetic, and production categories that it claims.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: ai-dynamo/aisim
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: dbee3c4
  • Layers: Repository YAML
  • Review profile: ASSERTIVE
  • Plan: Enterprise
  • Run ID: e3fbc066-c94d-415d-b0b0-62ac88203c14
Commits

Reviewing files that changed from the base of the PR and between 954544c and 608244e.

Files selected for processing: 5

  • docs/perf-model/fpm-self-service/implementation.md
  • python/aisim/collector/fpm_forward/database.py
  • python/aisim/collector/network/slurm/README.md
  • python/aisim/collector/sglang/README.md
  • python/aisim/tests/unit/collector/test_fpm_dense_synthetic_kv.py
Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Recent review details

Context from checks skipped due to timeout: 2

  • GitHub Check: build
  • GitHub Check: CodeRabbit
Additional context used

Code guidelines: 7

python/aisim/.claude/rules/collector/failure_handling.md — configured
python/aisim/.claude/rules/collector/layer_permissions.md — configured
python/aisim/.claude/rules/collector/overview.md — configured
python/aisim/.claude/rules/collector/case_authoring.md — configured
python/aisim/.claude/rules/perfmodel-api.md — auto-discovered
AGENTS.md — configured
REVIEW.md — configured
Path-based instructions: 10

Enforce the mapped collector guidelines (start from overview.md).

CodeRabbit configuration file

Files:

  • python/aisim/collector/fpm_forward/database.py
  • python/aisim/collector/network/slurm/README.md
  • python/aisim/collector/sglang/README.md
Check commands, defaults, supported runtimes, public names, and claims against executable behavior.

CodeRabbit configuration file

Files:

  • docs/perf-model/fpm-self-service/implementation.md
Read REVIEW.md before commenting.

CodeRabbit configuration file

Files:

  • docs/perf-model/fpm-self-service/implementation.md
  • python/aisim/collector/fpm_forward/database.py
  • python/aisim/collector/network/slurm/README.md
  • python/aisim/collector/sglang/README.md
  • python/aisim/tests/unit/collector/test_fpm_dense_synthetic_kv.py
Applicability reference: AGENTS.md: # AGENTS Before making any change under `python/aisim/collector/**` MUST read: `python/aisim/.claude/rules/collector/failure_handling.md` Source excerpt: **Observe, don't predict.**

CodeRabbit inference engine (python/aisim/.claude/rules/collector/failure_handling.md)

Files:

  • python/aisim/collector/fpm_forward/database.py
  • python/aisim/collector/network/slurm/README.md
  • python/aisim/collector/sglang/README.md
  • python/aisim/tests/unit/collector/test_fpm_dense_synthetic_kv.py
Applicability reference: AGENTS.md: # AGENTS Before making any change under `python/aisim/collector/**` MUST read: `python/aisim/.claude/rules/collector/layer_permissions.md` (layer Source excerpt: | Layer | Allowed | Forb...

CodeRabbit inference engine (python/aisim/.claude/rules/collector/layer_permissions.md)

Files:

  • python/aisim/collector/fpm_forward/database.py
  • python/aisim/collector/network/slurm/README.md
  • python/aisim/collector/sglang/README.md
  • python/aisim/tests/unit/collector/test_fpm_dense_synthetic_kv.py
Applicability reference: AGENTS.md: # AGENTS Before making any change under `python/aisim/collector/**` MUST read: `python/aisim/.claude/rules/collector/overview.md` (the five things — Source excerpt: Generate the same cas...

CodeRabbit inference engine (python/aisim/.claude/rules/collector/overview.md)

Files:

  • python/aisim/collector/fpm_forward/database.py
  • python/aisim/collector/network/slurm/README.md
  • python/aisim/collector/sglang/README.md
  • python/aisim/tests/unit/collector/test_fpm_dense_synthetic_kv.py
Applicability reference: AGENTS.md: # AGENTS Before making any change under `python/aisim/collector/**` MUST read: For case YAML and op-plan work: `python/aisim/.claude/rules/collector/case_authoring.md` Source excerpt: Ho...

CodeRabbit inference engine (python/aisim/.claude/rules/collector/case_authoring.md)

Files:

  • python/aisim/collector/fpm_forward/database.py
  • python/aisim/collector/network/slurm/README.md
  • python/aisim/collector/sglang/README.md
  • python/aisim/tests/unit/collector/test_fpm_dense_synthetic_kv.py
Applicability reference: AGENTS.md: # AGENTS Before changing a performance model, its configuration, or a caller in Rust, Python, CLI, Sweeper, Replay, or Planner, MUST read and follow [`perfmodel-api.md`](python/aisim/.cl...

CodeRabbit inference engine (python/aisim/.claude/rules/perfmodel-api.md)

Files:

  • python/aisim/tests/unit/collector/test_fpm_dense_synthetic_kv.py
Source excerpt: Before adding code or other content copied, adapted, translated, or substantially derived from an external project, MUST: Add an entry to the root `THIRD_PARTY_NOTICES.md` identifying the derived files, upstream source and i...

CodeRabbit inference engine (AGENTS.md)

Files:

  • docs/perf-model/fpm-self-service/implementation.md
  • python/aisim/collector/fpm_forward/database.py
  • python/aisim/collector/network/slurm/README.md
  • python/aisim/collector/sglang/README.md
  • python/aisim/tests/unit/collector/test_fpm_dense_synthetic_kv.py
Source excerpt: Python describes and orchestrates work.

CodeRabbit inference engine (REVIEW.md)

Files:

  • docs/perf-model/fpm-self-service/implementation.md
  • python/aisim/collector/fpm_forward/database.py
  • python/aisim/collector/network/slurm/README.md
  • python/aisim/collector/sglang/README.md
  • python/aisim/tests/unit/collector/test_fpm_dense_synthetic_kv.py
ast-grep (0.51.1)

python/aisim/tests/unit/collector/test_fpm_dense_synthetic_kv.py

[info] 140-140: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload)
Note: [CWE-116] Improper Encoding or Escaping of Output.

(use-jsonify)

[info] 142-142: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"schema": "dynamo.fpm.benchmark_worker_probe", "schema_version": 1})
Note: [CWE-116] Improper Encoding or Escaping of Output.

(use-jsonify)

[info] 152-152: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload)
Note: [CWE-116] Improper Encoding or Escaping of Output.

(use-jsonify)

Multi-repo context ai-dynamo/aiconfigurator, ai-dynamo/dynamo

Linked repositories findings

ai-dynamo/aiconfigurator (consumer and frozen compatibility reference)

Seed-regime classifier only approves one warm-up skip reason. approved_skip_reasons contains only moe_tp_balanced_by_construction [::ai-dynamo/aiconfigurator::collector/fpm_forward/database.py:168]. In _kv_seed_regime, a configuration-level skip_reason outside that set, with no kvwarm_real_kv or kvwarm_fake_fallback marker, returns fake_fallback [::ai-dynamo/aiconfigurator::collector/fpm_forward/database.py:175-186].

This matters for PR #529. Dynamo emits skip:dense_model_content_insensitive for a gate-rejected dense configuration with no stamp [::ai-dynamo/dynamo::components/src/dynamo/vllm/instrumented_scheduler.py:5825-5826]. The aiconfigurator classifier would label the same configuration fake_fallback unless the PR changes this path. I did not read the PR diff, so I cannot confirm whether it does.

Existing test asserts the fallback for non-approved skip reasons. test_non_protocol_kvwarm_skips_remain_fake_fallback is parametrized over skip_reason and asserts that every row is fake_fallback [::ai-dynamo/aiconfigurator::tests/unit/collector/test_fpm_forward.py:1349-1359]. The search output does not show the parameter values. If dense_model_content_insensitive is among them, this test directly conflicts with the PR's intended acceptance of native skip evidence and must be updated or reconciled.

Legacy stamp tests. Tests around kvwarm_fake_fallback markers appear at tests/unit/collector/test_fpm_forward.py:1291-1303 and :1340. The lines are under docstrings stating that warm-ineligible topologies mark every point as kvwarm_fake_fallback, which is the legacy path the PR keeps [::ai-dynamo/aiconfigurator::tests/unit/collector/test_fpm_forward.py:1291-1293]. I did not read their bodies.

Rust consumer keys on the fake_fallback label. In aic-core, fake_fallback is computed as kv_seed_regime == FPM_KV_SEED_FAKE_FALLBACK [::ai-dynamo/aiconfigurator::aic-core/rust/aiconfigurator-core/src/perf_database/fpm_forward.rs:705]. Replacement and keep-and-warn counts apply only to rows flagged fake_fallback [::ai-dynamo/aiconfigurator::aic-core/rust/aiconfigurator-core/src/perf_database/fpm_forward.rs:781-827]. A row labeled skip:dense_model_content_insensitive would therefore bypass healing and be counted as neither replaced nor kept. This is probably intended for legitimately synthetic-correct rows, but the label-to-behavior mapping should be confirmed. I did not see where unknown labels are validated.

Documentation and README. collector/README.md describes kv_seed_regime values as real_kv, fake_fallback, and others [::ai-dynamo/aiconfigurator::collector/README.md:163-165]. It should be checked against any new skip: label the PR accepts.

Named PR test files were not found. A search for test_fpm_dense_synthetic_kv.py, test_fpm_measurement_evidence.py, and test_fpm_worker_probe_artifact.py returned nothing. Only tests/unit/sdk/test_fpm_forward.py and tests/unit/collector/test_fpm_forward.py matched test_fpm_forward.py. The checkout I searched may not be the PR head, so this does not prove the files are absent.

ai-dynamo/dynamo (producer of the evidence stamps)

Stamp assignment. When the warm-up gate rejects the whole configuration (warm_eligible is False), no stamp is written and the point's regime comes from the skip reason. Otherwise the point gets kvwarm_fake_fallback [::ai-dynamo/dynamo::components/src/dynamo/vllm/instrumented_scheduler.py:7486-7496]. The comment names dense_model_content_insensitive and dataset_empty as example reasons [::ai-dynamo/dynamo::components/src/dynamo/vllm/instrumented_scheduler.py:7487-7491].

Regime helper. _kvwarm_seed_regime returns skip:<reason> for a gate-rejected configuration with no stamp. It returns skip:None in the non-warm-up path that reads skip_reason from _kvwarm_meta [::ai-dynamo/dynamo::components/src/dynamo/vllm/instrumented_scheduler.py:5822-5827]. The helper's docstring says fake_fallback is the injection-stamp result [::ai-dynamo/dynamo::components/src/dynamo/vllm/instrumented_scheduler.py:5796].

Documented contract. The artifact docs state that decode rows carry skip:<reason> and that fake_fallback occurs only outside attention data parallelism. They give skip:dense_model_content_insensitive as meaning synthetic KV is correct by construction. Under attention data parallelism, ranks whose check passed record skip:peer_ineligible [::ai-dynamo/dynamo::docs/fern/pages/reference/observability/environment-variables.mdx:357]. Dynamo's skip: labels are therefore the producer-side contract the PR consumes. Any change to that contract would break the aiconfigurator side.

Not verified

  • I did not read the PR diff, so I cannot tell whether it changes _kv_seed_regime or the Rust loader.
  • The related PR in ai-dynamo/aisim#778 (reported in PR context) changes the same eligibility path to read decoder configs under llm_config as well as text_config. I did not search aisim, so this coordination note is unverified.

Reviewers should confirm that the approved-skip-reason set and the Rust fake_fallback handling match the native skip: labels that Dynamo emits, and that the parametrized test at test_fpm_forward.py:1349 does not conflict with the accepted reasons.

@jasonqinzhou jasonqinzhou left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval from the AISimulate PR Review Monitor, enabled by Jason.

Reviewed commit: 8d6274611c10af602db70fdcab46a00a400b6e8b

9.3/10 · high confidence

Accept Dynamo's explicit per-point dense KV skip marker during formal FPM aggregation, alongside the historical fallback stamp. The PR retains the existing eligibility checks and documents and tests both producer formats.

9.3/10, APPROVE. The change consumes the validated per-point regime through the existing dense full-attention, runtime, and topology gate. No actionable defect was found, and exact-head hosted Fast CI and Full CI passed. Local native-extension limitations are documented in the verification and residual risks.

* ci: switch workflows to renamed AISim runner pools

Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com>

* ci: recognize AISim runner labels in actionlint

Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com>

* ci: use the ARC job-pod workspace for HOME

Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com>

* ci: add containerized canaries for the runner cutover

Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com>

* ci: activate controlled canaries after runner registration

Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com>

* ci: respect the shared runner execution contract

Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com>

---------

Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com>
(cherry picked from commit 976d75a)
Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com>
@saturley-hall
saturley-hall requested review from a team as code owners November 13, 2026 20:49
@copy-pr-bot

copy-pr-bot Bot commented Nov 15, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants